Update circle. #270
Update circle. #270
Conversation
|
@Marc-Andre-Rivet Could you perhaps review this if you have time? I think you know this stuff pretty well! |
| version: 2 | ||
|
|
||
| jobs: | ||
| "python-2.7": &test-template |
There was a problem hiding this comment.
Not aware of this syntax '&test-template', you use it to refer to this job configuration for python36 and 37 so you can just override what you need?
There was a problem hiding this comment.
Yes, I took that from plotly/dash#222.
.circleci/config.yml
Outdated
| - run: | ||
| name: Run tests | ||
| command: | | ||
| npm run test |
There was a problem hiding this comment.
Not important, but you can do 'npm test' instead
package.json
Outdated
| @@ -17,17 +17,25 @@ | |||
| "publish-all": "npm publish && python setup.py sdist upload", | |||
| "publish-pypi": "npm run prepublish && python setup.py sdist && twine upload --sign --skip-existing", | |||
| "start": "./node_modules/.bin/builder run build-dev", | |||
| "test": "./node_modules/.bin/eslint --fix .", | |||
| "test": "./node_modules/.bin/eslint --fix src", | |||
There was a problem hiding this comment.
I find it weird that test is used to fix linting errors. Should probably be a script of its own like 'fixlint', 'lint.fix' or something along those lines.
You should be able to call 'eslint --fix src' directly instead too, same for builder above.
I would expect eslint to be in the list of dependencies if we are calling it in our scripts. We should not trust babel-eslint to pull it along.
| }, | ||
| "author": "Chris Parmer <[email protected]>", | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "babel-core": "^6.26.3", |
There was a problem hiding this comment.
Do we have a shrinkwrap or package-lock file for this project? If we do not we should, as it makes the version of the dependencies downloaded by our users more predictable and saves us from weird compatibility issues that may arise from various technically-compatible but not identical versions.
| } | ||
| ] | ||
| } | ||
| }; |
There was a problem hiding this comment.
What's the reason for this new webpack.config.js file? The webpack config stuff for the components is in ./config/webpack/, won't it be confusing to have 2 config files for webpack?
There was a problem hiding this comment.
The old configs were for webpack 3, this is for webpack 4.
|
@valentijnnieman Yes we should add the prettier here, Its in the list plotly/dash#328 |
|
@valentijnnieman I have added prettier and fixed the errors it was reporting, for the tabs file I put a eslint-disable at the top because it was reporting a bunch of prop types error for the EnhancedTab. Please review. |
|
@T4rk1n Oh yeah |
|
💃 |
Update the config of circleci to version 2
Builderwas failing, added the configs ofdash-component-boilerplate.The test_link_scroll
dash-core-components/test/test_integration.py
Lines 604 to 616 in a727cde